fixes hip-tests using system rocm installation with TheRock#3880
fixes hip-tests using system rocm installation with TheRock#3880tcgu-amd wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts hip-tests’ ROCm path resolution so builds under TheRock don’t unintentionally pick up a system /opt/rocm installation, ensuring the build uses the intended ROCm/HIP from the build environment.
Changes:
- Gate the default
ROCM_PATH=/opt/rocmbehavior behind a TheRock-specific condition. - Preserve fallback to
HIP_PATHwhenROCM_PATHis missing or invalid.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| if(DEFINED THEROCK_SUPERPROJECT_INCLUDE_DIRS) | ||
| # When building under TheRock superproject, default ROCM_PATH to HIP_PATH | ||
| set(ROCM_PATH ${HIP_PATH}) |
There was a problem hiding this comment.
This doesn't seem like the right fix. In general, subprojects shouldn't need to know anything about a particular superproject like TheRock.
We need to rip out all hardcoded uses of /opt/rocm/. See also discussion at ROCm/TheRock#3825
There was a problem hiding this comment.
@ScottTodd Thanks for looping me into the discussion! In that case, for this particular fix, we could simply just remove the condition and always set ROCM_PATH to HIP_PATH. TheRock check was added only in the spirit of being conservative and preserving the original /opt/rocm build logic.
|
Closing in favor of more systematic refactor. |
Motivation
hip-tests picks up system rocm installation when building with TheRock because it hard sets ROCM_PATH env variable to /opt/rocm
Technical Details
In CMakeLists.txt, ROCM_PATH is hard-coded to be set to /opt/rocm when not defined. Since TheRock always unsets ROCM_PATH for all subprojects, this leads to ROCM_PATH always picking up the system installation as long as there is one.
JIRA ID
N/A
Test Plan
Ran locally to confirm behavior.
Test Result
Passed.
Submission Checklist